Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MODLISTS-57] Soft deletion within lists #57

Merged
merged 9 commits into from
Jan 25, 2024
Merged

[MODLISTS-57] Soft deletion within lists #57

merged 9 commits into from
Jan 25, 2024

Conversation

ncovercash
Copy link
Member

@ncovercash ncovercash commented Jan 24, 2024

Jira MODLISTS-57

Purpose

Lists should be able to be soft deleted and later accessible via the main /lists endpoint with special query parameter includeDeleted.

Deleted lists should no longer be accessible via /lists/{id}/*.

Out of scope

  • Returning special error responses when operations are attempted on deleted lists; currently, we just return 404 as though the list never existed/was hard deleted.
  • Hard deleting or un-deleting a list

import org.folio.list.domain.dto.ListUpdateRequestDTO;
import org.folio.list.exception.AbstractListException;
import org.folio.list.rest.UsersClient.User;
import org.folio.list.util.TaskTimer;

@Data
@With
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generates methods like withIsDeleted, withName, etc., which clones and edits one field in a fluent manner.

Copy link
Collaborator

@mweaver-ebsco mweaver-ebsco Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally out of scope and I'm not asking for any changes, but I just need to complain...

ngl, I really hate everything about this class, but I don't care enough to go out of my way to change it... It's like a perfect storm of things that annoy me: odd class name + lombok + ORM + mutability + leaky abstraction (it has a dependency on something in UserClient) + business logic in a data class (with no comments, no less)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. We could do a quick clean up noise-wise at least by removing the @Column's (it'll derive from fields), and changing some to primitives (removes @NotNull's) — super minor change, but would save a smidge of sanity

import java.util.UUID;

@Repository
public interface ListRepository extends CrudRepository<ListEntity, UUID>, PagingAndSortingRepository<ListEntity, UUID> {

Optional<ListEntity> findByIdAndIsDeletedFalse(UUID id);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spring derives the query for us

@ncovercash ncovercash changed the title [MODLISTS-71] Soft deletion within lists [MODLISTS-57] Soft deletion within lists Jan 24, 2024
Copy link

sonarcloud bot commented Jan 24, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
91.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@ncovercash ncovercash merged commit d9dd46f into master Jan 25, 2024
6 checks passed
@ncovercash ncovercash deleted the modlists-71 branch January 25, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants